Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add support for dynamic slideshow #411

Merged
merged 1 commit into from
Mar 21, 2024
Merged

add support for dynamic slideshow #411

merged 1 commit into from
Mar 21, 2024

Conversation

RyanCoulsonCA
Copy link
Member

@RyanCoulsonCA RyanCoulsonCA commented Mar 13, 2024 •
edited
Loading

Related Item(s)

#410 and #353

Changes

  • The slideshow panel now supports all panel types instead of just images.
  • The charts panel no longer contains a carousel. Chart galleries should be replaced with a slideshow (see breaking changes section below).
  • Text panels in the slideshow will display a scrollbar if they are too long.
  • As per Aleks' feedback here, I have removed the ability to switch between slides by dragging the mouse in order to allow the user to pan the map and to copy text if they'd like.

Breaking Changes

  • As discussed here, I have removed the carousel from the chart panel. To fix existing chart galleries, change the type from chart to slideshow, change the charts property to items, and add type: "chart" to each of the entries (it now treats each of the charts as their own chart panel).

For example, if this is what your current chart panel config looks like:

{
  "type": "chart",
  "charts": [
    {
      "src": "00000000-0000-0000-0000-000000000000/charts/en/chartConfig.json"
    },
    {
      "src": "00000000-0000-0000-0000-000000000000/charts/en/Total releases of ethylene glycol for 2019, by province_en.csv",
      "options": {
        "title": "Figure 2: Total releases of ethylene glycol for 2019, by province",
        "xAxisLabel": "Quantity (tonnes)",
        "subtitle": "",
        "type": "bar"
      }
    },
    {
      "src": "00000000-0000-0000-0000-000000000000/charts/en/EG_releases_2019_en.csv",
      "options": {
        "title": "Figure 1: Percentage of total ethylene glycol releases for 2019, by sector",
        "subtitle": "",
        "type": "pie"
      }
    }
  ]
}

Replace it with the following:

{
  "type": "slideshow",
  "items": [
    {
      "type": "chart",
      "src": "00000000-0000-0000-0000-000000000000/charts/en/chartConfig.json"
    },
    {
      "type": "chart",
      "src": "00000000-0000-0000-0000-000000000000/charts/en/Total releases of ethylene glycol for 2019, by province_en.csv",
      "options": {
        "title": "Figure 2: Total releases of ethylene glycol for 2019, by province",
        "xAxisLabel": "Quantity (tonnes)",
        "subtitle": "",
        "type": "bar"
      }
    },
    {
      "type": "chart",
      "src": "00000000-0000-0000-0000-000000000000/charts/en/EG_releases_2019_en.csv",
      "options": {
        "title": "Figure 1: Percentage of total ethylene glycol releases for 2019, by sector",
        "subtitle": "",
        "type": "pie"
      }
    }
  ]
}
  • To fix existing image slideshows, simply replace the images property with items.

Testing

Play around with the following and ensure everything works as expected (will add links once the demo is up):

  1. Click on the Charts Panel link in the Dynamic Panel Test slide. Ensure these charts load and can be switched.
  2. Text, map, and chart panels have been added to the slideshow in the NPRI substances reported for oil sands mining facilities slide.
  3. The image slideshow in the Reported mine tailings from oil sands surface mining facilities slide hasn't been touched. Ensure it still works as normal.
  4. There is a chart slideshow on the Chart Gallery slide that you can play around with as well.

This change is Reviewable

@github-actions GitHub Actions
Copy link

Your demo site is ready! 🚀 Visit it here: https://ramp4-pcar4.github.io/story-ramp/fix-410/#/en/00000000-0000-0000-0000-000000000000

yileifeng
yileifeng reviewed Mar 15, 2024
View reviewed changes
Copy link
Member

@yileifeng yileifeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed here, I have removed the carousel from the chart panel. To fix existing chart galleries, change the type from chart to slideshow, change the charts property to items, and add type: "chart" to each of the entries (it now treats each of the charts as their own chart panel).

Definitely should bring this up again in next week's meeting. It seems like chart slideshows are all cut-off on mobile view but this issue appears on the main build as well so can be new issue.

Reviewable status: 0 of 7 files reviewed, all discussions resolved

yileifeng
yileifeng approved these changes Mar 15, 2024
View reviewed changes
Copy link
Member

@yileifeng yileifeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @RyanCoulsonCA)

szczz
szczz approved these changes Mar 18, 2024
View reviewed changes
Copy link
Member

@szczz szczz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @RyanCoulsonCA)

KashishMistry
KashishMistry approved these changes Mar 21, 2024
View reviewed changes
Copy link
Member

@KashishMistry KashishMistry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @RyanCoulsonCA)

@yileifeng yileifeng merged commit 209174b into ramp4-pcar4:main Mar 21, 2024
3 checks passed
@RyanCoulsonCA RyanCoulsonCA mentioned this pull request Mar 21, 2024
Merged
@dan-bowerman dan-bowerman mentioned this pull request Mar 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KashishMistry KashishMistry KashishMistry approved these changes

@yileifeng yileifeng yileifeng approved these changes

@szczz szczz szczz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@RyanCoulsonCA @szczz @yileifeng @KashishMistry